archive: Prevent symlink-directory collision chmod attack (#442)
authorAlex Crichton <alex@alexcrichton.com>
Thu, 19 Mar 2026 21:58:05 +0000 (16:58 -0500)
committerFabian Grünbichler <debian@fabian.gruenbichler.email>
Thu, 26 Mar 2026 10:27:54 +0000 (11:27 +0100)
When unpacking a tarball containing a symlink followed by a directory
entry with the same path, unpack_dir previously used fs::metadata()
which follows symlinks. This allowed an attacker to modify permissions
on arbitrary directories outside the extraction path.

The fix uses fs::symlink_metadata() to detect symlinks and refuse to
treat them as valid existing directories.

Document more exhaustively+consistently security caveats.

Reported-by: Sergei Zimmerman <https://github.com/xokdvium>
Assisted-by: OpenCode (Claude claude-opus-4-5)
Signed-off-by: Colin Walters <walters@verbum.org>
Co-authored-by: Colin Walters <walters@verbum.org>
FG: drop test-related changes
Signed-off-by: Fabian Grünbichler <debian@fabian.gruenbichler.email>
Fixes: CVE-2026-33056
Gbp-Pq: Topic vendor
Gbp-Pq: Name tar-CVE-2026-33056.patch

vendor/tar-0.4.44/src/archive.rs
vendor/tar-0.4.44/src/entry.rs

index 4d569c63595cfe47743eba0fd2aa32bac520215b..459c28b653440ab8ab9ff28155fc281065149efc 100644 (file)
@@ -93,9 +93,21 @@ impl<R: Read> Archive<R> {
     /// extracting each file in turn to the location specified by the entry's
     /// path name.
     ///
-    /// This operation is relatively sensitive in that it will not write files
-    /// outside of the path specified by `dst`. Files in the archive which have
-    /// a '..' in their path are skipped during the unpacking process.
+    /// # Security
+    ///
+    /// A best-effort is made to prevent writing files outside `dst` (paths
+    /// containing `..` are skipped, symlinks are validated). However, there
+    /// have been historical bugs in this area, and more may exist. For this
+    /// reason, when processing untrusted archives, stronger sandboxing is
+    /// encouraged: e.g. the [`cap-std`] crate and/or OS-level
+    /// containerization/virtualization.
+    ///
+    /// If `dst` does not exist, it is created. Unpacking into an existing
+    /// directory merges content. This function assumes `dst` is not
+    /// concurrently modified by untrusted processes. Protecting against
+    /// TOCTOU races is out of scope for this crate.
+    ///
+    /// [`cap-std`]: https://docs.rs/cap-std/
     ///
     /// # Examples
     ///
index 843719f0fad135ccfb012afbc4ef91fe3cc6301a..6898b0abdd7bc721f9a108905e3d84e110603b9c 100644 (file)
@@ -212,8 +212,9 @@ impl<'a, R: Read> Entry<'a, R> {
     /// also be propagated to the path `dst`. Any existing file at the location
     /// `dst` will be overwritten.
     ///
-    /// This function carefully avoids writing outside of `dst`. If the file has
-    /// a '..' in its path, this function will skip it and return false.
+    /// # Security
+    ///
+    /// See [`Archive::unpack`].
     ///
     /// # Examples
     ///
@@ -446,7 +447,7 @@ impl<'a> EntryFields<'a> {
         // If the directory already exists just let it slide
         fs::create_dir(dst).or_else(|err| {
             if err.kind() == ErrorKind::AlreadyExists {
-                let prev = fs::metadata(dst);
+                let prev = fs::symlink_metadata(dst);
                 if prev.map(|m| m.is_dir()).unwrap_or(false) {
                     return Ok(());
                 }